Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake: only add ui tools and their dependencies in toplevel build #137

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

wirew0rm
Copy link
Member

When adding gr-digitizers to a build using add_subdirectory or fetch_content, the targets for the ui tools and dependencies can clash. This commit removes them from the build if this project is not the top-level cmake project.

Also default disables gr-digitizer tests in that case by default.

Longer term we should improve the cmake in all projects to allow overriding dependencies and correctly use cmake namespaces for the targets...

When adding gr-digitizers to a build using add_subdirectory or
fetch_content, the targets for the ui tools and dependencies can clash.
This commit removes them from the build if this project is not the
too-level cmake project.

Also default disables gr-digitizer tests in that case by default.
Copy link
Member

@RalphSteinhagen RalphSteinhagen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should invest into detecting whether a cmake project is a top-level project or referenced to suppress these transient dependencies being pulled in. The proposed solutions is good for now though.

@RalphSteinhagen RalphSteinhagen merged commit 8b0ea27 into dev-prototype Jan 18, 2024
6 checks passed
@RalphSteinhagen RalphSteinhagen deleted the reduceDepsForSubmoduleUse branch January 18, 2024 09:10
Copy link

sonarcloud bot commented Jan 18, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants